Skip to content

Fix large string handling in CLUSTER|NODES and CLUSTER|SHARDS#1858

Merged
kevin-montrose merged 6 commits into
mainfrom
users/kmontrose/clusterNodesLargeResponseFix
Jun 8, 2026
Merged

Fix large string handling in CLUSTER|NODES and CLUSTER|SHARDS#1858
kevin-montrose merged 6 commits into
mainfrom
users/kmontrose/clusterNodesLargeResponseFix

Conversation

@kevin-montrose

Copy link
Copy Markdown
Contributor

Same issue experimentally identified in #1781 and fixed for INFO in #1776.

…s; another example of the issue we should cleanup up re. #1781
Copilot AI review requested due to automatic review settings June 5, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to fix the “large response spins forever” pattern (previously addressed for INFO in #1776) for cluster responses, specifically CLUSTER NODES and CLUSTER SHARDS, by introducing a path that can emit outputs larger than the fixed send buffer without relying on while (!TryWriteX(...)) SendAndReset() loops that may never make progress.

Changes:

  • Adds RespWriteUtils.TryWriteBulkStringLength(int len, ...) to allow writing just the $<len>\r\n prefix without requiring the full payload to fit.
  • Switches CLUSTER NODES and CLUSTER SHARDS handlers to use a new helper intended to stream large outputs.
  • Introduces WriteAsciiLargeRespString in the cluster session to chunk output (using ArrayPool<byte> + ASCII encoding) when it does not fit in the current send buffer.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
libs/common/RespWriteUtils.cs Adds an overload to write a bulk-string length header from an int length.
libs/cluster/Session/RespClusterBasicCommands.cs Reworks CLUSTER NODES/SHARDS response writing to use a new “large output” helper.

Comment thread libs/cluster/Session/RespClusterBasicCommands.cs
Comment thread libs/cluster/Session/RespClusterBasicCommands.cs
Comment thread libs/cluster/Session/RespClusterBasicCommands.cs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread libs/cluster/Session/RespClusterBasicCommands.cs
Comment thread libs/cluster/Session/RespClusterBasicCommands.cs
Comment thread libs/cluster/Session/RespClusterBasicCommands.cs
@kevin-montrose kevin-montrose merged commit c0a07db into main Jun 8, 2026
145 checks passed
@kevin-montrose kevin-montrose deleted the users/kmontrose/clusterNodesLargeResponseFix branch June 8, 2026 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants